elementary/messages/messaging_integrations/slack_web.py (1)

110-127: Joining path can never retry: _handle_send_err always raises, preventing resend

As implemented, _handle_send_err ends with an unconditional MessagingIntegrationError, so _send_message’s retry path (return self._send_message(...)) is never reached—even after successfully joining a channel. This breaks the intended recover-on-join flow.

Suggested fix (outside changed hunk): only return after handling the recoverable case; raise for unrecoverable cases:

def _handle_send_err(self, err: SlackApiError, channel_name: str):
    if self.tracking:
        self.tracking.record_internal_exception(err)
    err_type = err.response.data.get("error")

    if err_type == "not_in_channel":
        logger.info(f'Elementary app is not in the channel "{channel_name}". Attempting to join.')
        channel_id = self._get_channel_id(channel_name)
        self._join_channel(channel_id=channel_id)
        logger.info(f"Joined channel {channel_name}")
        return  # allow caller to retry send
    elif err_type == "channel_not_found":
        raise MessagingIntegrationError(
            f"Channel {channel_name} was not found by the Elementary app. Please add the app to the channel."
        )

    # For all other errors, propagate a failure (no retry)
    raise MessagingIntegrationError(f"Failed to send a message to channel - {channel_name}: {err_type}")

elementary/messages/messaging_integrations/slack_web.py (1)

158-161: Optional: guard against duplicate loads in concurrent calls

If multiple threads/processes call _ensure_channels_loaded concurrently, you may end up running _load_channels multiple times. Not harmful, but avoidable with a simple lock.

Outside this hunk, consider:

# at top of file
import threading

# in __init__
self._channels_cache_lock = threading.Lock()

# here
def _ensure_channels_loaded(self) -> None:
    if self._channels_cache is None:
        with self._channels_cache_lock:
            if self._channels_cache is None:
                self._load_channels()

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

elementary/messages/messaging_integrations/slack_web.py (1)

109-127: Fix control flow: don’t raise after successful join; return to allow retry

After handling "not_in_channel" and joining, the function still hits the unconditional raise below, preventing the retry in _send_message from ever executing. Early-return after a successful join and only raise for unknown errors.

Apply this diff:

     if err_type == "not_in_channel":
         logger.info(
             f'Elementary app is not in the channel "{channel_name}". Attempting to join.'
         )
-        channel_id = self._get_channel_id(channel_name, only_public=True)
+        channel_id = self._get_channel_id(channel_name, only_public=True)
         self._join_channel(channel_id=channel_id)
         logger.info(f"Joined channel {channel_name}")
-    elif err_type == "channel_not_found":
+        return
+    elif err_type == "channel_not_found":
         raise MessagingIntegrationError(
             f"Channel {channel_name} was not found by the Elementary app. Please add the app to the channel."
         )
-    raise MessagingIntegrationError(
-        f"Failed to send a message to channel - {channel_name}"
-    )
+    else:
+        raise MessagingIntegrationError(
+            f"Failed to send a message to channel - {channel_name}"
+        )

elementary/messages/messaging_integrations/slack_web.py (1)

148-153: Handle channel IDs and normalized names; current logic can fail for IDs or “#name” inputs

If destination is already a channel ID (e.g., "C…" or "G…"), the current equality check against channel["name"] will never match, causing a false "not found" and breaking the not_in_channel join path. Also, comparisons should accept "#name" and use name_normalized for resilience (Slack lowercase normalization).

Apply this diff:

-    def _get_channel_id(self, channel_name: str, only_public: bool = False) -> str:
-        for channel in self._iter_channels(only_public=only_public):
-            if channel["name"] == channel_name:
-                return channel["id"]
-        raise MessagingIntegrationError(f"Channel {channel_name} not found")
+    def _get_channel_id(self, channel_name: str, only_public: bool = False) -> str:
+        # Fast-path: if already a conversation ID, return it as-is.
+        if channel_name and channel_name[0] in ("C", "G") and channel_name[1:].isalnum():
+            return channel_name
+        target = channel_name.lstrip("#").lower()
+        for channel in self._iter_channels(only_public=only_public):
+            name = channel.get("name_normalized") or channel.get("name")
+            if name and name.lower() == target:
+                return channel.get("id")  # Slack guarantees 'id' here
+        raise MessagingIntegrationError(f"Channel {channel_name} not found")

Run this to locate all call sites and ensure the updated signature (only_public) is accounted for where behavior matters:

#!/bin/bash
# Find all usages of _get_channel_id across the repo with context.
rg -nP --type=python -C3 '\b_get_channel_id\s*\('

# Additionally, check for places passing channel IDs directly into send paths
# to validate that the short-circuit will cover them.
rg -nP --type=python -C2 'chat_postMessage\([^)]*channel\s*=\s*([\'"]).*?\1'

elementary/messages/messaging_integrations/slack_web.py (1)

146-146: Prefer keyword args in recursive call to avoid param order regressions

Minor readability/robustness improvement: pass cursor and only_public by name to make future signature changes safer and clearer.

-            yield from self._iter_channels(next_cursor, only_public)
+            yield from self._iter_channels(cursor=next_cursor, only_public=only_public)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.